Skip to content

Release build resources early in non-watch mode and prevent process hang on unhandled error - #33715

Closed
alan-agius4 wants to merge 3 commits into
angular:mainfrom
alan-agius4:perf-release-compilation-resources-non-watch
Closed

Release build resources early in non-watch mode and prevent process hang on unhandled error#33715
alan-agius4 wants to merge 3 commits into
angular:mainfrom
alan-agius4:perf-release-compilation-resources-non-watch

Conversation

@alan-agius4

@alan-agius4 alan-agius4 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

This PR combines two related improvements to resource lifecycle management and error handling in @angular/build:application:

1. Early Resource Release in Non-Watch Mode

  • Disposes angularCompilationContext, typescriptContexts, otherContexts, componentStyleBundler, and codeBundleCache early in non-watch mode once bundling completes.
  • Reclaims memory held by TypeScript compilation worker pool threads (ParallelCompilation), AST caches (SourceFileCache, MemoryLoadResultCache), and ESBuild child processes before post-bundle steps and disk writing.
  • Made AngularCompilationContext.dispose() and ExecutionResult.dispose() memoized and idempotent so subsequent disposal calls safely share #disposal promises.
  • Closes Production build peak memory +53% after upgrading @angular/build 21.2.13 → 22.0.0 (same code, same esbuild version) #33368

2. Process Hang Prevention on Unhandled Build Errors (#33716)

  • Wraps executeBuild() in a comprehensive try...catch block and instantiates executionResult immediately after bundlerContexts is created, guaranteeing that all ESBuild child processes, TypeScript compiler worker threads, and stylesheet bundlers are disposed if an error is thrown at any stage of the build.
  • Fixed a SQLite handle leak in I18nInliner.close() by closing this.#cache (angular-i18n.db) alongside worker pool destruction.
  • Ensures Sass workers are shut down in build-action even if an initial --watch build throws an unhandled exception.
  • Closes @angular/build: process hangs instead of exiting on a thrown post-bundle error (result never disposed) #33716

Note

Reviewer Note: Since wrapping executeBuild() in a try...catch block indents the function body, it is recommended to review this PR with "Hide whitespace" enabled in GitHub diff settings for easier review.

@angular-robot angular-robot Bot added area: performance Issues related to performance area: @angular/build labels Jul 31, 2026
@alan-agius4
alan-agius4 requested a review from dgp1130 July 31, 2026 09:07
@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Jul 31, 2026
gemini-code-assist[bot]

This comment was marked as outdated.

@alan-agius4
alan-agius4 force-pushed the perf-release-compilation-resources-non-watch branch from aecad28 to 3709966 Compare July 31, 2026 09:14
Disposes the Angular compilation and bundler contexts within the build execution
for non-watch builds once bundling has completed. This terminates the TypeScript
compilation worker and JavaScript transformation workers before the post-bundle
steps (chunk optimization, i18n inlining, index generation) execute instead of
after all results have been emitted. Repeat compilation disposal calls now return
the in progress disposal to ensure all callers can await completion of the
underlying compiler shutdown. The result disposal also releases the incremental
build caches which can retain the emitted contents of every TypeScript file
within the program. This reduces the peak memory usage of the build for both
the non-watch early disposal and watch mode teardown.

Closes angular#33368
@alan-agius4
alan-agius4 force-pushed the perf-release-compilation-resources-non-watch branch from 3709966 to ef75f6a Compare July 31, 2026 09:14
@alan-agius4 alan-agius4 reopened this Jul 31, 2026
When an unhandled exception is thrown during a build (such as IndexHtmlGenerator.readIndex throwing when an index HTML file is missing or unreadable), executeBuild() previously re-threw without calling .dispose() on executionResult or angularCompilationContext. Consequently, open ESBuild child processes, TypeScript compiler worker threads, and SQLite database connections prevented the Node.js process from exiting.

This commit addresses the issue by:
1. Wrapping the entire body of executeBuild() in a try...catch block and instantiating executionResult immediately after bundlerContexts is created, so any thrown error reliably disposes all worker pools and AST caches before re-throwing.
2. Closing the SQLite cache store (angular-i18n.db) in I18nInliner.close() alongside worker pool destruction.
3. Ensuring Sass workers are shut down in build-action even if an initial --watch build throws an exception.

Closes angular#33716
@alan-agius4
alan-agius4 force-pushed the perf-release-compilation-resources-non-watch branch from 7ffbbac to c0cf4a5 Compare July 31, 2026 12:20
@alan-agius4 alan-agius4 changed the title perf(@angular/build): release build resources early in non-watch mode perf(@angular/build): release build resources early in non-watch mode and prevent process hang on unhandled error Jul 31, 2026
@alan-agius4

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request improves resource management and cleanup during the build process. Key changes include disposing of bundler rebuild contexts, Angular compilation contexts, and caches early in non-watch mode or when a build fails. Additionally, disposal methods across various classes (such as AngularCompilationContext and ExecutionResult) have been updated to cache and reuse their disposal promises to prevent redundant teardown operations. A test was also updated to ensure the build terminates cleanly when a non-existent index HTML file is provided. There are no review comments, so I have no feedback to provide.

@alan-agius4 alan-agius4 changed the title perf(@angular/build): release build resources early in non-watch mode and prevent process hang on unhandled error Release build resources early in non-watch mode and prevent process hang on unhandled error Jul 31, 2026
// This is fire-and-forget so worker teardown runs concurrently in the background
// without blocking the critical path of post-bundle optimization steps.
// Any in-flight disposal is awaited in the builder action's finally block.
void Promise.allSettled([angularCompilationContext?.dispose(), executionResult.dispose()]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider: Do we need Promise.allSettled if we're not await-ing it? Can we just do:

void angularCompilationContext?.dispose();
void executeResult.dispose();

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! Since we aren't awaiting it here (and any in-flight disposal promises are awaited in the builder action's finally block), Promise.allSettled is unnecessary. Updated to call .dispose() on each directly.

Comment on lines +33 to +39
dispose(): void {
this.clear();
this.modifiedFiles.clear();
this.typeScriptFileCache.clear();
this.loadResultCache.clear();
this.referencedFiles = undefined;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider: This looks like it's just clearing maps, not really releasing any resources. Do we need this dispose, or could we just have the consumer of this class drop its reference to this SourceFileCache object? Wouldn't that have the same GC effect?

OTOH, "clearing the cache" while still keep a reference to that cache does seem like a reasonable feature, so maybe I'm just getting hung up on the name dispose? The class is still valid after being disposed right? Would clear be a better name to communicate that?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great observation. In non-watch mode, ExecutionResult is returned by executeBuild() and passed to post-bundling steps (such as index HTML generation, service worker generation, and disk writing) while holding a reference to codeBundleCache. Clearing the cache here immediately releases cached TypeScript ASTs and emitted file contents before those post-bundling steps execute without requiring callers to drop their reference to ExecutionResult.

Renamed this method to override clear() (and added a super.clear() call) to better communicate that it clears all cached entries across the internal maps while leaving the cache instance valid for potential reuse.

alan-agius4 added a commit to alan-agius4/angular-cli that referenced this pull request Jul 31, 2026
- Replace unnecessary Promise.allSettled with direct dispose calls in execute-build.ts
- Rename SourceFileCache dispose() to override clear() and call super.clear() to clear base Map alongside internal cache structures
@alan-agius4
alan-agius4 force-pushed the perf-release-compilation-resources-non-watch branch from e3ad0d6 to b3fe12b Compare July 31, 2026 17:31
@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer action: merge The PR is ready for merge by the caretaker labels Jul 31, 2026
@alan-agius4
alan-agius4 force-pushed the perf-release-compilation-resources-non-watch branch from b3fe12b to e934d32 Compare July 31, 2026 17:48
@angular angular deleted a comment from ngbot Bot Jul 31, 2026
@alan-agius4

Copy link
Copy Markdown
Collaborator Author

This PR was merged into the repository. The changes were merged into the following branches:

alan-agius4 added a commit that referenced this pull request Jul 31, 2026
…#33715)

Disposes the Angular compilation and bundler contexts within the build execution
for non-watch builds once bundling has completed. This terminates the TypeScript
compilation worker and JavaScript transformation workers before the post-bundle
steps (chunk optimization, i18n inlining, index generation) execute instead of
after all results have been emitted. Repeat compilation disposal calls now return
the in progress disposal to ensure all callers can await completion of the
underlying compiler shutdown. The result disposal also releases the incremental
build caches which can retain the emitted contents of every TypeScript file
within the program. This reduces the peak memory usage of the build for both
the non-watch early disposal and watch mode teardown.

Closes #33368

PR Close #33715
alan-agius4 added a commit that referenced this pull request Jul 31, 2026
…33715)

When an unhandled exception is thrown during a build (such as IndexHtmlGenerator.readIndex throwing when an index HTML file is missing or unreadable), executeBuild() previously re-threw without calling .dispose() on executionResult or angularCompilationContext. Consequently, open ESBuild child processes, TypeScript compiler worker threads, and SQLite database connections prevented the Node.js process from exiting.

This commit addresses the issue by:
1. Wrapping the entire body of executeBuild() in a try...catch block and instantiating executionResult immediately after bundlerContexts is created, so any thrown error reliably disposes all worker pools and AST caches before re-throwing.
2. Closing the SQLite cache store (angular-i18n.db) in I18nInliner.close() alongside worker pool destruction.
3. Ensuring Sass workers are shut down in build-action even if an initial --watch build throws an exception.

Closes #33716

PR Close #33715
alan-agius4 added a commit that referenced this pull request Jul 31, 2026
…33715)

When an unhandled exception is thrown during a build (such as IndexHtmlGenerator.readIndex throwing when an index HTML file is missing or unreadable), executeBuild() previously re-threw without calling .dispose() on executionResult or angularCompilationContext. Consequently, open ESBuild child processes, TypeScript compiler worker threads, and SQLite database connections prevented the Node.js process from exiting.

This commit addresses the issue by:
1. Wrapping the entire body of executeBuild() in a try...catch block and instantiating executionResult immediately after bundlerContexts is created, so any thrown error reliably disposes all worker pools and AST caches before re-throwing.
2. Closing the SQLite cache store (angular-i18n.db) in I18nInliner.close() alongside worker pool destruction.
3. Ensuring Sass workers are shut down in build-action even if an initial --watch build throws an exception.

Closes #33716

PR Close #33715
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker area: @angular/build area: performance Issues related to performance target: patch This PR is targeted for the next patch release

Projects

None yet

2 participants